Set up tilt CI for testing gitops-stack #268
Conversation
📝 WalkthroughWalkthroughAdds a GitHub Actions "Checks" workflow running Tilt CI in a docker/tilt container; introduces a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Cont as Runner Container\n(docker/tilt:latest)
participant CTL as ctlptl / Kind
participant Helm as Helm (azure/setup-helm)
participant Tilt as Tilt (tilt ci)
GH->>Cont: start workflow (checkout)
Cont->>CTL: create Kind cluster via ctlptl
CTL-->>Cont: cluster ready
Cont->>Helm: install Helm
Helm-->>Cont: ready
Cont->>Tilt: run `tilt ci` (apply Tiltfile)
Tilt-->>Cont: render/apply resources (namespaces, traefik, workloads)
Cont->>GH: report workflow status
rect rgb(220,238,255)
note right of Tilt: Tiltfile now declares `traefik` namespace\nand updated resource dependencies
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/worklows/checks.yml
🔇 Additional comments (4)
.github/worklows/checks.yml (4)
1-6: LGTM!The workflow name and pull request trigger configuration for the develop branch are correct.
15-16: LGTM!The checkout action version is current and appropriate.
18-19: LGTM - assuming Docker access is resolved.The ctlptl command syntax is correct for creating a Kind cluster with a registry. This step will work once Docker daemon access is properly configured (as noted in the earlier comment).
21-22: LGTM!The
tilt cicommand is appropriate for running automated Tilt tests in CI.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/checks.yml (1)
12-23: Critical: Container cannot access Docker daemon; ctlptl will fail.This is the same critical issue previously identified. The workflow configuration has two blocking problems:
No Docker daemon access: GitHub-hosted runners don't provide Docker socket access to container jobs by default. The
ctlptl create cluster kindcommand at line 20 requires Docker to create the Kind cluster and will fail.Missing ctlptl: The
docker/tilt:latestimage is not a maintained multi-tool container and does not bundle ctlptl. The official Tilt distribution is via CLI binaries.Recommended solution: Remove the container configuration and run directly on the
ubuntu-latestrunner, which has Docker pre-installed. Then explicitly install ctlptl and tilt:🔎 Proposed fix
jobs: checks: - runs-on: ubuntu-latest - container: - image: docker/tilt:latest - + steps: - uses: actions/checkout@v4 - + + - name: Install ctlptl + run: | + CTLPTL_VERSION="0.8.34" + curl -fsSL https://github.com/tilt-dev/ctlptl/releases/download/v${CTLPTL_VERSION}/ctlptl.${CTLPTL_VERSION}.linux.x86_64.tar.gz | sudo tar -xzv -C /usr/local/bin ctlptl + + - name: Install Tilt + run: curl -fsSL https://raw.githubusercontent.com/tilt-dev/tilt/master/scripts/install.sh | bash + - name: Create k8s Kind Cluster run: ctlptl create cluster kind --registry=ctlptl-registry - + - name: Test Using Local Config run: tilt ci - -Verify the latest versions of ctlptl and tilt if needed:
What is the latest stable version of ctlptl?What is the recommended installation method for Tilt CLI in CI environments?
🧹 Nitpick comments (1)
.github/workflows/checks.yml (1)
3-7: Consider aligning trigger branch filters.The
pushtrigger runs on all branches whilepull_requestis restricted todevelop. This asymmetry means the workflow executes on pushes to any branch but only on PRs targeting develop.If the intent is to test only develop-related changes, consider adding the same branch filter to
push:on: push: branches: - develop pull_request: branches: - develop
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ory-hydra/templates/init-clients.yaml (1)
68-73: Critical: Shell syntax error whentokenEndpointAuthMethodis set butowneris not.The trailing backslash on Line 69 creates a shell syntax error when
.tokenEndpointAuthMethodis defined but.owneris not. In this scenario, the rendered command ends with:--token-endpoint-auth-method "value" \The dangling backslash with no continuation line causes the shell to fail.
🔎 Proposed fix
Remove the trailing backslash from Line 69 since it's conditionally the last parameter:
{{- if .tokenEndpointAuthMethod }} - --token-endpoint-auth-method "{{ .tokenEndpointAuthMethod }}" \ + --token-endpoint-auth-method "{{ .tokenEndpointAuthMethod }}" {{- end }}Alternatively, if you need the backslash for cases where
.ownerfollows, you could add the backslash conditionally:{{- if .tokenEndpointAuthMethod }} - --token-endpoint-auth-method "{{ .tokenEndpointAuthMethod }}" \ + --token-endpoint-auth-method "{{ .tokenEndpointAuthMethod }}"{{- if .owner }} \{{- end }} {{- end }}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/checks.ymlTiltfileapps/ory-hydra/templates/init-clients.yamlcluster/namespaces.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/checks.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Sheikah45
Repo: FAForever/gitops-stack PR: 268
File: .github/workflows/checks.yml:9-14
Timestamp: 2025-12-23T18:21:25.226Z
Learning: The docker/tilt:latest image is a comprehensive CI image that includes tilt, ctlptl, Docker CLI, and related Kubernetes tooling, making it suitable for GitHub Actions container jobs that need to create Kind clusters.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: checks
- GitHub Check: checks
🔇 Additional comments (3)
Tiltfile (2)
159-159: LGTM! Correctly tracks the new traefik namespace.The addition of
"traefik:namespace"properly registers the new namespace with Tilt's resource tracking, aligning with the namespace definition added incluster/namespaces.yaml.
185-185: LGTM! Proper resource dependency ensures correct ordering.Adding
resource_deps=["namespaces"]ensures thattraefik-setupwaits for all namespaces (including the new traefik namespace) to be created before attempting to deploy Traefik resources. This prevents potential race conditions and follows the same dependency pattern used by other resources in the Tiltfile.cluster/namespaces.yaml (1)
18-22: LGTM! Clean addition of the traefik namespace.The new
traefikNamespace is properly defined with correct YAML structure and delimiter. This aligns with the Tiltfile changes that reference and depend on this namespace, establishing a dedicated namespace for Traefik infrastructure components separate from application namespaces.
Brutus5000
force-pushed
the
develop
branch
19 times, most recently
from
0085ff9 to
4abaeae
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Tiltfile (1)
240-244: Good refactoring! Consider simplifying the dependency chain.The website deployment flow refactoring improves code clarity by separating the build, patch, and apply steps. The addition of the
wordpressdependency is correct.Since
wordpressalready depends ontraefik(line 286), thefaf-websiteresource could list only["wordpress"]as its dependency instead of["traefik", "wordpress"]. The current implementation is correct but slightly redundant.🔎 Optional simplification
-k8s_resource(workload="faf-website", objects=["faf-website:ingressroute"], resource_deps=["traefik", "wordpress"], labels=["website"], links=[link("https://www.localhost", "FAForever Website")]) +k8s_resource(workload="faf-website", objects=["faf-website:ingressroute"], resource_deps=["wordpress"], labels=["website"], links=[link("https://www.localhost", "FAForever Website")])
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/checks.ymlREADME.MDTiltfileapps/faf-icebreaker/templates/config.yamlapps/faf-icebreaker/templates/deployment.yamlapps/faf-user-service/templates/deployment.yamlapps/ory-hydra/templates/init-clients.yamlcluster/namespaces.yamlinfra/mariadb/values.yamltilt/scripts/helm-with-cache.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/faf-user-service/templates/deployment.yaml
- cluster/namespaces.yaml
- .github/workflows/checks.yml
- apps/faf-icebreaker/templates/deployment.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Sheikah45
Repo: FAForever/gitops-stack PR: 268
File: .github/workflows/checks.yml:9-14
Timestamp: 2025-12-23T18:21:33.542Z
Learning: The docker/tilt:latest image is a comprehensive CI image that includes tilt, ctlptl, Docker CLI, and related Kubernetes tooling, making it suitable for GitHub Actions container jobs that need to create Kind clusters.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: checks
- GitHub Check: checks
🔇 Additional comments (6)
apps/faf-icebreaker/templates/config.yaml (1)
12-12: LGTM! DB_NAME addition is consistent.The new
DB_NAMEentry aligns with the database name inDB_URL(line 13) and matches the username pattern. This properly coordinates with the MariaDB configuration ininfra/mariadb/values.yaml.infra/mariadb/values.yaml (1)
53-58: LGTM! Icebreaker database entry is properly configured.The new
databasesAndUsersentry correctly references thefaf-icebreakerConfigMap and Secret, with keys that match the configuration added inapps/faf-icebreaker/templates/config.yaml. The structure follows the established pattern used by other services in the file.apps/ory-hydra/templates/init-clients.yaml (1)
69-74: LGTM! Shell continuation bug fix.This correctly fixes a shell script line-continuation issue in the Helm template. Previously, if multiple optional parameters at the end (like both
.tokenEndpointAuthMethodand.owner) were defined, the command would break because the first parameter lacked a trailing backslash. The new approach ensures proper multi-line continuation by:
- Adding backslashes to all conditional parameters
- Using a terminating semicolon that's always present
The semicolon safely terminates the command regardless of which optional parameters are rendered.
README.MD (1)
82-82: LGTM! Clear documentation improvement.The updated reference to the external test data source makes it easier for developers to locate and understand the test data being used.
Tiltfile (2)
159-159: LGTM! Correct namespace addition.The addition of
traefik:namespaceproperly extends namespace management to include the traefik infrastructure component.
185-185: LGTM! Essential dependency for reliable startup.Adding the
namespacesdependency ensures traefik-setup waits for the namespace to be created first, preventing race conditions during CI runs.
Closes #31
Use tilt ci command to run tests of the infrastructure in an automated fashion.
tilt ci works by running the TiltFile specified and waiting for all the configured resources to start up. If any resource fails then the test fails. This takes into account all the health checks for the various services as well. More can be found at tilt.dev
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.